Implement Rename transform #15
Conversation
|
Ooof, my bad for half of these things. I guess that what you get from sending a PR of after-midnight code without a review. I will fix the formatting and naming right away, but that I am not 100% sure about is how to phase out |
|
Is it as simple as looping through the results of |
|
Exactly, you can write it in a single: newcols = map(cols) do col
# do the renaming
endSee the other transforms for examples. |
|
I'm sorry, I screwed up git a bit. I'll comment when it's ready for review. |
|
I believe its ready for another review. |
|
@Omar-Elrefaei did you have time to update this PR? |
|
Unfortunately no, I haven't been getting a chance to continue the work.
…On Thu, Nov 18, 2021, 9:06 AM Júlio Hoffimann ***@***.***> wrote:
@Omar-Elrefaei <https://github.com/Omar-Elrefaei> did you have time to
update this PR?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIXXL6RMMX5QOWLZOVZCS3UMUB5NANCNFSM5IACTAYQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
@Omar-Elrefaei you mean that this PR should be closed? |
No, if not today after my dayjob, I'll definitely be sending updates in the weekend. |
Codecov Report
@@ Coverage Diff @@
## master #15 +/- ##
==========================================
+ Coverage 88.16% 91.44% +3.28%
==========================================
Files 13 14 +1
Lines 321 339 +18
==========================================
+ Hits 283 310 +27
+ Misses 38 29 -9
Continue to review full report at Codecov.
|
|
I think it's ready for another review.
|
|
Awesome improvements @Omar-Elrefaei , I am merging it. The test that is failing is unrelated. For some reason one of our visual tests is failing randomly. Do you think you can take a look at this issue? Maybe we are missing some |
|
Great! I'll open an issue so that not to forget to investigate the failing tests. |
Omar-Elrefaei
changed the title
This should be fully functional, but I think the usage of
push!!will need to be redesigned to be more functional and efficient like other transforms. That is the place where I think I need a little help: I am not sure how to make it so that I don't needpush!!and use𝒯 = (; zip(anames, acols)...)instead like the rest of the of the transforms.